New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not allow TypeCastExpressions w/o parens #8956
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9334/ |
e6a6ed3
to
b3f3918
Compare
b3f3918
to
dc022cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of duplicating parseExpressionParenItem
/parseParenItem
and the check in toReferencedList
/parseParenAndDistinguishExpression
, I think that it would be easier to pass an optional boolean isParenthesizedExpression
parameter to toReferencedList
. We should then disallow type casts if !isParenthesizedExpression || length > 1
.
@@ -1932,36 +1961,16 @@ export default (superClass: Class<Parser>): Class<Parser> => | |||
for (let i = 0; i < exprList.length; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we can use for/of here, since we compile it using the assuneArray
option (for (a of b)
-> for (let i = 0; i < b.length ..
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I leave that for now, i don't want to make the commit diff bigger. Can do that in a separate PR.
dc022cc
to
70a9fae
Compare
@nicolo-ribaudo Thanks for the very helpful input. I changed it and it is a lot cleaner and easier now, and still works the same. 👍 |
70a9fae
to
72b1f0a
Compare
This adds an additional check to disallow flow type casts without parens inside of sequence expressions.
It also enables another check which was already present but never executed.
It is technically a breaking change, but as flow does not allow this, it is probably safe to assume nobody is using this. Or are we now being more strict?
I also tried using babel/babylon#446 (which also solves this problems), but the detection of arrow functions had lots of edge cases and ambiguities and overall seemed harder to maintain than the current parsing of arrows.